Migrate internal apis to environment component #9291
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PerfectSlayer
added
comp: core
PerfectSlayer
commented
Aug 1, 2025
| return Stream.of("TMP", "TEMP", "USERPROFILE") | ||
| .map(EnvironmentVariables::get) | ||
| .filter(Objects::nonNull) | ||
| .filter(((Predicate<String>) String::isEmpty).negate()) |
There was a problem hiding this comment.
📝 notes: This behavior change (empty --> !empty) is expected. It’s a fix to the PidHelper on Windows platform and was confirmed with the author first.
There was a problem hiding this comment.
Maybe add some comment about that?
There was a problem hiding this comment.
For clarification, what kind of comment do you expect here?
One of the next step for the environment component will be to handle the "temp dir" computation.
So feedback about it would be useful for this part too 👍
There was a problem hiding this comment.
Comment that will explain this non obvious empty --> !empty on Windows.
|
Code coverage: total 57.35%, base diff 0.01%, patch 32.61% (view details) This comment will be updated automatically if new data arrives.🔗 Commit SHA: f17f23d | Docs | Was this helpful? Give us feedback! |
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 9 metrics, 6 unstable metrics. See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (333.331 µs) : 276, 391
. : milestone, 333,
basic (279.563 µs) : 273, 286
. : milestone, 280,
loop (8.969 ms) : 8964, 8975
. : milestone, 8969,
section candidate
noprobe (314.144 µs) : 288, 340
. : milestone, 314,
basic (277.345 µs) : 271, 284
. : milestone, 277,
loop (8.976 ms) : 8971, 8982
. : milestone, 8976,
|
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 45 metrics, 14 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.049 s) : 0, 1049251
Total [baseline] (8.609 s) : 0, 8609078
Agent [candidate] (1.042 s) : 0, 1042053
Total [candidate] (8.577 s) : 0, 8576719
section iast
Agent [baseline] (1.174 s) : 0, 1174373
Total [baseline] (9.267 s) : 0, 9267309
Agent [candidate] (1.182 s) : 0, 1181700
Total [candidate] (9.306 s) : 0, 9305784
gantt
title insecure-bank - break down per module: candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.436 ms) : 0, 1436
crashtracking [candidate] (1.413 ms) : 0, 1413
BytebuddyAgent [baseline] (735.034 ms) : 0, 735034
BytebuddyAgent [candidate] (729.656 ms) : 0, 729656
GlobalTracer [baseline] (242.646 ms) : 0, 242646
GlobalTracer [candidate] (241.285 ms) : 0, 241285
AppSec [baseline] (30.112 ms) : 0, 30112
AppSec [candidate] (30.062 ms) : 0, 30062
Debugger [baseline] (6.057 ms) : 0, 6057
Debugger [candidate] (5.993 ms) : 0, 5993
Remote Config [baseline] (647.762 µs) : 0, 648
Remote Config [candidate] (647.514 µs) : 0, 648
Telemetry [baseline] (12.301 ms) : 0, 12301
Telemetry [candidate] (12.122 ms) : 0, 12122
section iast
crashtracking [baseline] (1.432 ms) : 0, 1432
crashtracking [candidate] (1.439 ms) : 0, 1439
BytebuddyAgent [baseline] (848.05 ms) : 0, 848050
BytebuddyAgent [candidate] (853.907 ms) : 0, 853907
GlobalTracer [baseline] (232.32 ms) : 0, 232320
GlobalTracer [candidate] (233.252 ms) : 0, 233252
IAST [baseline] (29.786 ms) : 0, 29786
IAST [candidate] (27.466 ms) : 0, 27466
AppSec [baseline] (27.278 ms) : 0, 27278
AppSec [candidate] (27.477 ms) : 0, 27477
Debugger [baseline] (5.709 ms) : 0, 5709
Debugger [candidate] (8.259 ms) : 0, 8259
Remote Config [baseline] (581.261 µs) : 0, 581
Remote Config [candidate] (578.513 µs) : 0, 579
Telemetry [baseline] (8.32 ms) : 0, 8320
Telemetry [candidate] (8.272 ms) : 0, 8272
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.046 s) : 0, 1046265
Total [baseline] (10.784 s) : 0, 10783647
Agent [candidate] (1.058 s) : 0, 1058242
Total [candidate] (10.807 s) : 0, 10807084
section appsec
Agent [baseline] (1.22 s) : 0, 1219873
Total [baseline] (10.886 s) : 0, 10885745
Agent [candidate] (1.227 s) : 0, 1227208
Total [candidate] (10.864 s) : 0, 10863808
section iast
Agent [baseline] (1.177 s) : 0, 1177240
Total [baseline] (10.892 s) : 0, 10892177
Agent [candidate] (1.177 s) : 0, 1177131
Total [candidate] (10.905 s) : 0, 10904859
section profiling
Agent [baseline] (1.191 s) : 0, 1190562
Total [baseline] (10.871 s) : 0, 10870567
Agent [candidate] (1.19 s) : 0, 1189670
Total [candidate] (10.857 s) : 0, 10856675
gantt
title petclinic - break down per module: candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.422 ms) : 0, 1422
crashtracking [candidate] (1.452 ms) : 0, 1452
BytebuddyAgent [baseline] (733.361 ms) : 0, 733361
BytebuddyAgent [candidate] (741.2 ms) : 0, 741200
GlobalTracer [baseline] (242.579 ms) : 0, 242579
GlobalTracer [candidate] (244.833 ms) : 0, 244833
AppSec [baseline] (30.278 ms) : 0, 30278
AppSec [candidate] (30.473 ms) : 0, 30473
Debugger [baseline] (6.075 ms) : 0, 6075
Debugger [candidate] (6.117 ms) : 0, 6117
Remote Config [baseline] (660.331 µs) : 0, 660
Remote Config [candidate] (660.222 µs) : 0, 660
Telemetry [baseline] (10.833 ms) : 0, 10833
Telemetry [candidate] (12.292 ms) : 0, 12292
section appsec
crashtracking [baseline] (1.425 ms) : 0, 1425
crashtracking [candidate] (1.442 ms) : 0, 1442
BytebuddyAgent [baseline] (751.934 ms) : 0, 751934
BytebuddyAgent [candidate] (758.133 ms) : 0, 758133
GlobalTracer [baseline] (235.617 ms) : 0, 235617
GlobalTracer [candidate] (236.255 ms) : 0, 236255
IAST [baseline] (23.734 ms) : 0, 23734
IAST [candidate] (23.735 ms) : 0, 23735
AppSec [baseline] (169.643 ms) : 0, 169643
AppSec [candidate] (169.314 ms) : 0, 169314
Debugger [baseline] (6.551 ms) : 0, 6551
Debugger [candidate] (7.27 ms) : 0, 7270
Remote Config [baseline] (616.254 µs) : 0, 616
Remote Config [candidate] (630.953 µs) : 0, 631
Telemetry [baseline] (9.286 ms) : 0, 9286
Telemetry [candidate] (9.212 ms) : 0, 9212
section iast
crashtracking [baseline] (1.436 ms) : 0, 1436
crashtracking [candidate] (1.443 ms) : 0, 1443
BytebuddyAgent [baseline] (849.806 ms) : 0, 849806
BytebuddyAgent [candidate] (849.572 ms) : 0, 849572
GlobalTracer [baseline] (232.476 ms) : 0, 232476
GlobalTracer [candidate] (233.177 ms) : 0, 233177
IAST [baseline] (31.533 ms) : 0, 31533
IAST [candidate] (28.378 ms) : 0, 28378
AppSec [baseline] (24.501 ms) : 0, 24501
AppSec [candidate] (28.805 ms) : 0, 28805
Debugger [baseline] (7.582 ms) : 0, 7582
Debugger [candidate] (5.731 ms) : 0, 5731
Remote Config [baseline] (591.796 µs) : 0, 592
Remote Config [candidate] (595.162 µs) : 0, 595
Telemetry [baseline] (8.381 ms) : 0, 8381
Telemetry [candidate] (8.321 ms) : 0, 8321
section profiling
crashtracking [baseline] (1.417 ms) : 0, 1417
crashtracking [candidate] (1.395 ms) : 0, 1395
BytebuddyAgent [baseline] (758.44 ms) : 0, 758440
BytebuddyAgent [candidate] (757.891 ms) : 0, 757891
GlobalTracer [baseline] (220.873 ms) : 0, 220873
GlobalTracer [candidate] (221.159 ms) : 0, 221159
AppSec [baseline] (29.819 ms) : 0, 29819
AppSec [candidate] (29.865 ms) : 0, 29865
Debugger [baseline] (6.32 ms) : 0, 6320
Debugger [candidate] (6.269 ms) : 0, 6269
Remote Config [baseline] (661.076 µs) : 0, 661
Remote Config [candidate] (718.168 µs) : 0, 718
Telemetry [baseline] (15.226 ms) : 0, 15226
Telemetry [candidate] (15.906 ms) : 0, 15906
ProfilingAgent [baseline] (108.494 ms) : 0, 108494
ProfilingAgent [candidate] (107.159 ms) : 0, 107159
Profiling [baseline] (109.129 ms) : 0, 109129
Profiling [candidate] (107.786 ms) : 0, 107786
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 12 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section baseline
no_agent (4.377 ms) : 4329, 4425
. : milestone, 4377,
iast (9.587 ms) : 9421, 9753
. : milestone, 9587,
iast_FULL (14.108 ms) : 13827, 14389
. : milestone, 14108,
iast_GLOBAL (10.344 ms) : 10143, 10546
. : milestone, 10344,
profiling (9.125 ms) : 8975, 9274
. : milestone, 9125,
tracing (7.592 ms) : 7480, 7704
. : milestone, 7592,
section candidate
no_agent (4.477 ms) : 4419, 4535
. : milestone, 4477,
iast (9.292 ms) : 9140, 9444
. : milestone, 9292,
iast_FULL (14.461 ms) : 14178, 14743
. : milestone, 14461,
iast_GLOBAL (10.011 ms) : 9823, 10199
. : milestone, 10011,
profiling (9.283 ms) : 9115, 9451
. : milestone, 9283,
tracing (7.818 ms) : 7707, 7928
. : milestone, 7818,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section baseline
no_agent (36.917 ms) : 36613, 37221
. : milestone, 36917,
appsec (47.753 ms) : 47321, 48184
. : milestone, 47753,
code_origins (45.587 ms) : 45205, 45969
. : milestone, 45587,
iast (46.486 ms) : 46077, 46895
. : milestone, 46486,
profiling (49.4 ms) : 48971, 49828
. : milestone, 49400,
tracing (43.583 ms) : 43221, 43944
. : milestone, 43583,
section candidate
no_agent (36.791 ms) : 36493, 37088
. : milestone, 36791,
appsec (47.196 ms) : 46774, 47618
. : milestone, 47196,
code_origins (44.574 ms) : 44196, 44953
. : milestone, 44574,
iast (45.058 ms) : 44680, 45437
. : milestone, 45058,
profiling (49.063 ms) : 48606, 49521
. : milestone, 49063,
tracing (44.17 ms) : 43813, 44527
. : milestone, 44170,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section baseline
no_agent (15.325 s) : 15325000, 15325000
. : milestone, 15325000,
appsec (14.905 s) : 14905000, 14905000
. : milestone, 14905000,
iast (18.496 s) : 18496000, 18496000
. : milestone, 18496000,
iast_GLOBAL (18.151 s) : 18151000, 18151000
. : milestone, 18151000,
profiling (15.299 s) : 15299000, 15299000
. : milestone, 15299000,
tracing (15.161 s) : 15161000, 15161000
. : milestone, 15161000,
section candidate
no_agent (14.954 s) : 14954000, 14954000
. : milestone, 14954000,
appsec (14.986 s) : 14986000, 14986000
. : milestone, 14986000,
iast (18.348 s) : 18348000, 18348000
. : milestone, 18348000,
iast_GLOBAL (18.233 s) : 18233000, 18233000
. : milestone, 18233000,
profiling (15.389 s) : 15389000, 15389000
. : milestone, 15389000,
tracing (15.079 s) : 15079000, 15079000
. : milestone, 15079000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.53.0-SNAPSHOT~f17f23d937, baseline=1.53.0-SNAPSHOT~0f3ab90a6f
dateFormat X
axisFormat %s
section baseline
no_agent (1.473 ms) : 1462, 1485
. : milestone, 1473,
appsec (3.67 ms) : 3453, 3887
. : milestone, 3670,
iast (2.198 ms) : 2135, 2260
. : milestone, 2198,
iast_GLOBAL (2.239 ms) : 2175, 2302
. : milestone, 2239,
profiling (2.065 ms) : 2013, 2117
. : milestone, 2065,
tracing (2.016 ms) : 1967, 2064
. : milestone, 2016,
section candidate
no_agent (1.476 ms) : 1465, 1488
. : milestone, 1476,
appsec (3.583 ms) : 3371, 3796
. : milestone, 3583,
iast (2.2 ms) : 2138, 2263
. : milestone, 2200,
iast_GLOBAL (2.24 ms) : 2178, 2303
. : milestone, 2240,
profiling (2.057 ms) : 2005, 2109
. : milestone, 2057,
tracing (2.02 ms) : 1971, 2069
. : milestone, 2020,
|
PerfectSlayer
force-pushed
the
bbujon/environment-step1
branch
from
d6e835b to
c63c1f8
Compare
PerfectSlayer
requested review from
jpbempel,
nikita-tkachenko-datadog and
sezen-datadog
and removed request for
a team
PerfectSlayer
force-pushed
the
bbujon/environment-step2
branch
from
3d69117 to
2f67106
Compare
|
Stacked PR broke when rebasing from GH following the Gradle smoke test fix. |
AlexeyKuznetsov-DD
requested changes
Aug 1, 2025
Comment on lines
14
to
17
| public static CiEnvironment local() { | ||
| Map<String, String> env; | ||
| try { | ||
| env = System.getenv(); | ||
| } catch (SecurityException e) { | ||
| env = Collections.emptyMap(); | ||
| } | ||
| return new CiEnvironmentImpl(env); | ||
| } | ||
|
|
There was a problem hiding this comment.
Not 100% but maybe this class can be refactored to use datadog.environment.EnvironmentVariables instead of caching System.getenv() in private map?
There was a problem hiding this comment.
So far, I did not expose the "get all" API in the environment components.
So I only added safe access from the code that use it.
Do you think it would be better to offer the "get all" API in SystemProperties and EnvironmentVariable?
There was a problem hiding this comment.
Do you think it would be better to offer the "get all" API in SystemProperties and EnvironmentVariable?
I like this, since the same security exception could occur with get all
There was a problem hiding this comment.
Yes, as we have special dedicated classes to work with env and props, better to have one to rule them all :)
There was a problem hiding this comment.
I added new API to cover those use case.
They were part of another stacked PR, I don't know how they end up for review here.
It will increase the scope of this PR but it's needed to get approval 😅
Comment on lines
142
to
149
| Properties systemProperties = System.getProperties(); | ||
| for (Map.Entry<Object, Object> e : systemProperties.entrySet()) { | ||
| String propertyName = (String) e.getKey(); | ||
| Object propertyValue = e.getValue(); | ||
| if ((propertyName.startsWith(Config.PREFIX) | ||
| || propertyName.startsWith("datadog.slf4j.simpleLogger.defaultLogLevel")) | ||
| && propertyValue != null) { | ||
| propagatedSystemProperties.put(propertyName, propertyValue.toString()); |
There was a problem hiding this comment.
Same here, can it be refactored to use SystemProperties?
| if (systemProperties.containsKey( | ||
| propertyNameToSystemPropertyName( | ||
| CiVisibilityConfig.CIVISIBILITY_INJECTED_TRACER_VERSION))) { | ||
| if (SystemProperties.get(propertyNameToSystemPropertyName(CIVISIBILITY_INJECTED_TRACER_VERSION)) |
There was a problem hiding this comment.
Maybe this long SystemProperties.get(propertyNameToSystemPropertyName(..) can be part of SystemProperties with some shorter and more convenient method name?
Just thinking out loud, if we can improve readability here and similar places.
There was a problem hiding this comment.
That would bring config concern to the environment component.
I would rather avoiding it to keep bot separated.
| return Stream.of("TMP", "TEMP", "USERPROFILE") | ||
| .map(EnvironmentVariables::get) | ||
| .filter(Objects::nonNull) | ||
| .filter(((Predicate<String>) String::isEmpty).negate()) |
There was a problem hiding this comment.
Maybe add some comment about that?
PerfectSlayer
force-pushed
the
bbujon/environment-step1
branch
from
c63c1f8 to
3925256
Compare
PerfectSlayer
force-pushed
the
bbujon/environment-step2
branch
from
2f67106 to
29fea0d
Compare
PerfectSlayer
removed request for
a team,
jpbempel,
nikita-tkachenko-datadog and
sezen-datadog
mhlidd
approved these changes
Aug 4, 2025
PerfectSlayer
force-pushed
the
bbujon/environment-step2
branch
from
29fea0d to
7aef877
Compare
PerfectSlayer
force-pushed
the
bbujon/environment-step2
branch
from
7aef877 to
f17f23d
Compare
Comment on lines
+57
to
+66
| try { | ||
| Map<String, String> map = new HashMap<>(); | ||
| for (String propertyName : System.getProperties().stringPropertyNames()) { | ||
| map.put(propertyName, System.getProperty(propertyName)); | ||
| } | ||
| return unmodifiableMap(map); | ||
| } catch (SecurityException ignored) { | ||
| return emptyMap(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Just thinking out loud here...
Is that OK that we stop iterating on first SecurityException?
Would it make sense to test each property and ignore only those that failed?
But potentially that could be slow...
AlexeyKuznetsov-DD
approved these changes
Aug 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
This PR migrate internal-api module to the new environment component
Motivation
Additional Notes
Environment component related stacked PRs:
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: LANGPLAT-458